Skip to content

Release 1.2.0#25

Merged
lord63 merged 30 commits intomasterfrom
dev
Mar 21, 2026
Merged

Release 1.2.0#25
lord63 merged 30 commits intomasterfrom
dev

Conversation

@lord63
Copy link
Copy Markdown
Owner

@lord63 lord63 commented Mar 21, 2026

  1. drop python 2.x support,add 3.10+ support;
  2. add context manager support, thanks to @liiight ;
  3. migrate to uv and github aciton;

liiight and others added 26 commits August 2, 2017 14:30
To be able to run the test prior to installation, as part of building a distro package.
- Add spinner reentry context manager test
- Parametrize non-TTY stdout tests
Copilot AI review requested due to automatic review settings March 21, 2026 09:49
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the project for a 1.2.0 release by dropping legacy Python packaging/CI tooling, adding Python 3.10+ support, and introducing context-manager support for the spinner.

Changes:

  • Migrate packaging to pyproject.toml (Hatchling) and add uv.lock for reproducible dev installs.
  • Replace Travis/tox/dev-requirements with a GitHub Actions workflow using uv + ruff + pytest.
  • Add Spinner context-manager support and expand the test suite to cover TTY/non-TTY behavior.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
uv.lock Adds uv lockfile to pin dev tool/test dependencies.
pyproject.toml Introduces modern packaging metadata + Python >=3.10.
.github/workflows/ci.yml Adds CI using uv/ruff/pytest across Python 3.10–3.13.
pyspin/spin.py Removes Py2 compat, adds TTY detection + context manager, refactors spinner output behavior.
test_pyspin.py Adds tests for context manager, non-TTY suppression, and flushing behavior.
README.rst Documents context-manager usage.
MANIFEST.in Updates included files/excludes (but needs correction).
tox.ini Removed (legacy test runner).
.travis.yml Removed (legacy CI).
setup.py / setup.cfg / dev-requirements.txt Removed (legacy packaging/dev deps).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

MANIFEST.in Outdated
@@ -1,2 +1,4 @@
include README.md
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MANIFEST.in includes README.md, but this repository uses README.rst (and pyproject.toml also points to README.rst). Including a non-existent file can break/warn during source distribution builds; update the manifest to include the correct README file (or remove MANIFEST.in if it’s no longer used with Hatchling).

Suggested change
include README.md
include README.rst

Copilot uses AI. Check for mistakes.
README.rst Outdated
Comment on lines +68 to +72
from __future__ import print_function

import time

from pyspin.spin import Default, Spinner
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new context-manager example still includes from __future__ import print_function, but the project now requires Python >= 3.10. This import is unnecessary in supported versions and may confuse readers about supported Python versions; consider removing it from the added snippet.

Copilot uses AI. Check for mistakes.
pyspin/spin.py Outdated
def _start(self):
if _is_tty(sys.stdout):
self._stop_event = threading.Event()
self.spin_thread = threading.Thread(target=self._init_spin)
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spinner thread is created as a non-daemon thread. If a user starts a spinner and exits unexpectedly without calling __exit__/_stop(), this thread can keep the process alive. Consider setting daemon=True (and optionally a thread name) when constructing threading.Thread(...) to avoid hanging process shutdowns.

Suggested change
self.spin_thread = threading.Thread(target=self._init_spin)
self.spin_thread = threading.Thread(
target=self._init_spin,
daemon=True,
name="SpinnerThread",
)

Copilot uses AI. Check for mistakes.
pyproject.toml Outdated
@@ -0,0 +1,34 @@
[project]
name = "pyspin"
version = "1.1.1"
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The project is labeled as a "Release 1.2.0" in the PR metadata, but the packaged version here is still 1.1.1. If this PR is intended to publish 1.2.0, bump the version accordingly (and keep uv.lock / any other version references consistent).

Copilot uses AI. Check for mistakes.
… exit

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

ChangLog.md:9

  • The changelog mentions hardened TTY detection, but the new behavior also suppresses all spinner output (including custom ending like "DONE") when stdout isn’t a TTY. Since that’s a user-visible/breaking behavior change, it would help to call it out explicitly in this 1.2.0 entry (e.g., under change).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pyspin/spin.py Outdated
sys.stdout.flush()
time.sleep(0.1)
if is_tty:
while future.running():
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_spin drives the spinner loop with while future.running(). A concurrent.futures.Future can be in the PENDING state immediately after submit(), where running() is false even though the work hasn’t completed yet, causing the loop to skip and the ending to print before the wrapped function finishes. Use while not future.done() (or while not future.done(): ...) to reliably spin until completion.

Suggested change
while future.running():
while not future.done():

Copilot uses AI. Check for mistakes.
Makefile Outdated
Comment on lines 2 to 3
@py.test --pep8 -v test_pyspin.py pyspin/

Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test target still runs py.test --pep8, but this PR removes pytest-pep8 (and CI now uses pytest + ruff). As-is, make test will fail due to the unknown --pep8 option. Update the target to run uv run pytest -v test_pyspin.py (and optionally add a separate lint target for uv run ruff check .).

Suggested change
@py.test --pep8 -v test_pyspin.py pyspin/
@uv run pytest -v test_pyspin.py
lint:
@uv run ruff check .

Copilot uses AI. Check for mistakes.
Comment on lines 66 to 80
def test_make_spin():
@spin.make_spin(spin.Default, 'Downloading...')
def fake_download():
time.sleep(2)

fake_download()


def test_make_spin_with_args():
@spin.make_spin(spin.Default, 'Downloading...')
def fake_download(url, retry_times=3):
print("Downloading {0}, will retry {1} times".format(url, retry_times))
time.sleep(2)

fake_download("https://www.example.com/text.txt", retry_times=5)
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests still sleep for 2 seconds per case (and test_several_calls sleeps twice), which makes CI noticeably slower across the 4-version Python matrix. Consider reducing these waits (e.g., to a few hundred ms) or using a synchronization primitive (Event) so the spinner logic is exercised without long real-time delays.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

ChangLog.md:5

  • There is trailing whitespace at the end of this line (after the contributor handle). Please remove it to avoid unnecessary diffs and keep markdown clean.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +10 to +25
("Box1", spin.Box1),
("Box2", spin.Box2),
("Box3", spin.Box3),
("Box4", spin.Box4),
("Box5", spin.Box5),
("Box6", spin.Box6),
("Box7", spin.Box7),
("Spin1", spin.Spin1),
("Spin2", spin.Spin2),
("Spin3", spin.Spin3),
("Spin4", spin.Spin4),
("Spin5", spin.Spin5),
("Spin6", spin.Spin6),
("Spin7", spin.Spin7),
("Spin8", spin.Spin8),
("Spin9", spin.Spin9),
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STYLES uses alignment spacing after commas (e.g., ("Box1", spin.Box1)), which will be flagged by ruff (E241: multiple spaces after ',') since CI runs ruff check .. Please remove the extra alignment spaces (or run an auto-formatter) so the examples pass linting.

Suggested change
("Box1", spin.Box1),
("Box2", spin.Box2),
("Box3", spin.Box3),
("Box4", spin.Box4),
("Box5", spin.Box5),
("Box6", spin.Box6),
("Box7", spin.Box7),
("Spin1", spin.Spin1),
("Spin2", spin.Spin2),
("Spin3", spin.Spin3),
("Spin4", spin.Spin4),
("Spin5", spin.Spin5),
("Spin6", spin.Spin6),
("Spin7", spin.Spin7),
("Spin8", spin.Spin8),
("Spin9", spin.Spin9),
("Box1", spin.Box1),
("Box2", spin.Box2),
("Box3", spin.Box3),
("Box4", spin.Box4),
("Box5", spin.Box5),
("Box6", spin.Box6),
("Box7", spin.Box7),
("Spin1", spin.Spin1),
("Spin2", spin.Spin2),
("Spin3", spin.Spin3),
("Spin4", spin.Spin4),
("Spin5", spin.Spin5),
("Spin6", spin.Spin6),
("Spin7", spin.Spin7),
("Spin8", spin.Spin8),
("Spin9", spin.Spin9),

Copilot uses AI. Check for mistakes.
def show(name, frames):
s = spin.Spinner(frames)
print(name)
for i in range(50):
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop variable i is unused here; ruff/Pyflakes will flag this as F841 (assigned to but never used) in CI. Use _ (or otherwise use i) to avoid failing lint.

Suggested change
for i in range(50):
for _ in range(50):

Copilot uses AI. Check for mistakes.
@lord63 lord63 merged commit c9c3a56 into master Mar 21, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants